feat: add joinphrases module - #105
Conversation
TannPat
commented
Dec 3, 2025
TannPat
commented
- add database model and CRUD methods
- add commands for adding, getting, and deleting joinphrases
- add logic to send joinphrases on user join
- implement conditions of minimum time and messages since last JP per user per room
- add otherHandler to track messages for miscellaneous use
PartMan7
left a comment
PartMan7
left a comment
There was a problem hiding this comment.
Hi there's a couple things I've left comments on
Most of the stuff is done though, thanks! Tag me if you'd like me to implement some of the comments I've mentioned
| }, | ||
| userId: { | ||
| type: String, | ||
| default: toId(username), |
There was a problem hiding this comment.
The username here is the bot's username; that's probably not the default we want to go with - ideally this should be a function that takes the given username
| help: 'Joinphrases module!', | ||
| perms: ['room', 'driver'], | ||
| syntax: 'CMD', | ||
| aliases: ['joinphrase'], |
There was a problem hiding this comment.
aliases and name should be swapped here - the name is the full name of the command, while aliases are abbreviations for those
| aliases: ['joinphrase'], | ||
| categories: ['utility'], | ||
| extendedAliases: { | ||
| addjp: ['jp', 'new'], |
There was a problem hiding this comment.
Could you also add ajp, djp, and ejp? Those are currently the most commonly used ones
| const args: string[] = arg.split(',').map(s => s.trim()); | ||
| const username = args[0]; | ||
| const phrase = args[1]; |
There was a problem hiding this comment.
This is broken with commas (phrases with commas will only have the first bit)
| const args: string[] = arg.split(',').map(s => s.trim()); | |
| const username = args[0]; | |
| const phrase = args[1]; | |
| const [username, phrase] = arg.lazySplit(/\s*,\s*/, 1); |
See https://github.com/PartMan7/PartBot/blob/main/src/globals/prototypes.ts#L58 for reference
| } catch (e: unknown) { | ||
| message.reply(`${username} already has a joinphrase in ${message.target.title}...` as ToTranslate); | ||
| } |
There was a problem hiding this comment.
You don't need to catch explicitly - throwing an error is the preferred approach, since that automatically handles private replies and whatnot
| messageCount: number; // messages since last jp | ||
| lastTime: number; // timestamp of last jp | ||
| } | ||
| const jpStateMap: Partial<Record<string, jpState>> = {}; |
There was a problem hiding this comment.
This should be in @/cache instead of here
| export function otherHandler(message: PSMessage) { | ||
| if (message.isIntro) return; | ||
| if (!message.author || !message.author.userid || !message.target || message.author.id === message.parent.status.userid) return; | ||
| if (message.content.startsWith('|')) return; | ||
| const roomId = message.target.id; | ||
|
|
||
| for (const key in jpStateMap) { | ||
| if (!jpStateMap[key]) continue; | ||
|
|
||
| const rId = key.split('-')[1]; | ||
| if (rId !== roomId) continue; | ||
|
|
||
| jpStateMap[key]!.messageCount++; | ||
| } // increment message count for each joinphrase in the room | ||
| } | ||
|
|
There was a problem hiding this comment.
This should be in its own file; this file is about join/leave/rename events
| state = { messageCount: minimumMessages, lastTime: 0 }; | ||
| jpStateMap[key] = state; | ||
| } | ||
| const now = Date.now() / 1000; |
There was a problem hiding this comment.
Try to use milliseconds instead of seconds wherever possible
| for (const key in jpStateMap) { | ||
| if (!jpStateMap[key]) continue; | ||
|
|
||
| const rId = key.split('-')[1]; | ||
| if (rId !== roomId) continue; | ||
|
|
||
| jpStateMap[key]!.messageCount++; | ||
| } // increment message count for each joinphrase in the room |
There was a problem hiding this comment.
Performance wise this is a bit concerning; if there's a couple hundred JPs total, you're looping over every single one of them on every message
Instead, the state map should be grouped by rooms instead so you don't iterate over JPs that aren't for the room
| // (Kinda creepy name for the feature, but it CAN be used in creepy ways so make sure it's regulated!) | ||
|
|
||
| const userId = toId(user), | ||
| roomId = toId(room); |
There was a problem hiding this comment.
Why's room been cast to roomId here?
- add database model and CRUD methods - add commands for adding, getting, and deleting joinphrases - add logic to send joinphrases on user join
- add conditions of minimum time and messages since last JP per user per room - add otherHandler to track messages for miscellaneous use
ff79d8e to
9b10ad0
Compare
|
Alright, this looks good now. Thanks! |
2 participants
